Fixing scalar shape handling: treat (1,) as vectors #4214
Conversation
+ by using more pytest.mark.parametrize, the overall number of overall tests increases, but the result should be easier to diagnose + an informative assert error message was added
|
Thanks for taking the initiative @michaelosthege ! |
|
I think I ran into the same problem as @StephenHogg in #4211 EDIT: I removed the traceback, because it's now documented in #4219 I'm on a quite recent Theano-PyMC master branch (4249a0). (Maybe you are too, @StephenHogg?) @brandonwillard assuming that this is indeed due to a change in Theano-PyMC, do you know about this issue already? Any hints? |
Codecov Report
@@ Coverage Diff @@
## master #4214 +/- ##
==========================================
+ Coverage 87.95% 87.97% +0.02%
==========================================
Files 88 88
Lines 14499 14483 -16
==========================================
- Hits 12752 12741 -11
+ Misses 1747 1742 -5
|
|
As far as theano-pymc goes, here's what The error looks the same, yes. |
|
The key line is here: pymc3\model.py:1800: in __init__
self.tag.test_value = theano.compile.view_op(data).tag.test_valueWe recently introduced a change to Theano's test value handling that validates the test values up front. The fix requires that the PyMC3-assigned test values be cast to the appropriate types (e.g. |
michaelosthege
force-pushed
the
fix-4206
branch
from
b8849d2
to
2e38d89
Compare
|
@lucianopaz thanks. Only in |
|
You know it's a good PR when: |
Co-authored-by: Thomas Wiecki <[email protected]>
|
Thanks! |
|
can I work on this? |
Did you mean this: #3896 |
Breaking Changes
()is the only scalar shape.(1,)will become vector of length 1New Shape Examples
Normal("n", shape=None)is the same as the defaultNormal("n"). Here it is up to the distribution (most will default to())shape=()explicitly specifies scalar shapeshape=0orshape=(0,)is up for discussion (see below)shape=1is the same asshape=(1,)is a length 1 vectorOpen ToDos
hundreds ofa few tests. They must be fixed.VelueErrorwhen a user passesshape=(0,)/shape=0to a distributionsize=(0,)/size=0to.random()remains allowed - because numpy also allows it on the random number generatorRandomWalkdistributions. While the tests cover shapes, they don't necessarily catch problems like GaussianRandomWalk prior predictive is broken #3962 (they look fine, but most don't have a.random()implementation. See .random() methods missing from some timeseries distributions #4337 )re-running relevant notebooks (oh dear!)Let's do this on a separate PR